Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add linting #22

Merged
merged 2 commits into from
Jan 30, 2024
Merged

Add linting #22

merged 2 commits into from
Jan 30, 2024

Conversation

tillywoodfield
Copy link
Collaborator

  • Add black, isort, flake8
  • Format code to comply (mostly quotes and line lengths)
  • Add github actions workflow for lint checks

@Bjwebb
Copy link
Contributor

Bjwebb commented Jan 30, 2024

BTW, it's preferable to set a different author for the black reformatting, so that your name isn't next to many lines in git blame. Here's an example from another repo OpenDataServices/flatten-tool@a094fea

@tillywoodfield
Copy link
Collaborator Author

BTW, it's preferable to set a different author for the black reformatting, so that your name isn't next to many lines in git blame. Here's an example from another repo OpenDataServices/flatten-tool@a094fea

@Bjwebb that makes sense, thank you, I've updated the first commit to use that author

@odscjames
Copy link
Collaborator

flake8 iatidata/

Extremely pedantically, we have this in the github actions (using paths with all the lint tools) which is great but can it be in the readme too? Otherwise sometimes peoples virtual environments get processed too which takes a while and gives many false alerts. We can do that in another PR tho.

@odscjames odscjames merged commit 81215bf into codeforIATI:main Jan 30, 2024
2 checks passed
@tillywoodfield
Copy link
Collaborator Author

flake8 iatidata/

Extremely pedantically, we have this in the github actions (using paths with all the lint tools) which is great but can it be in the readme too? Otherwise sometimes peoples virtual environments get processed too which takes a while and gives many false alerts. We can do that in another PR tho.

@odscjames Please could you clarify, I had this in the readme, do you want me to add something else?

@tillywoodfield tillywoodfield deleted the add-linting branch January 30, 2024 14:49
@odscjames
Copy link
Collaborator

You totally did, I apologise! Not sure how I got that wrong.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants